-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduction of the lifting of multiplication matrices (over the rationals) #153
Conversation
This PR also fixes #144. |
src/msolve/msolve.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are there all these changes for spaces? These changes make it way harder to reproduce the history of specific parts of msolve.c
for, e.g., bug hunting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had difficulties with many conflicts when merging master branch with this one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do we do about this? The current status of the PR still changes nearly 3100 lines of code in msolve.c
mostly spaces are adjusted (I assume due to some new text editor settings?). This makes it harder to understand the origin of possible bugs found in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest to leave it as is. It was really a pain to fix all the merge conflicts I encountered.
What I can do is to add comments on the real changes in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well that's bad, but if you cannot resolve your git problems we have to live with it.
|
This brings some significant speedups on several families of systems, including standard benchmarks (more than 1.5 on katsura-n, more than 2 on eco-n problems, speedups increase with n).
However, it uses more memory. This is why this option is not activated by default.